Private declarations should come after public.

As our internal guidelines mention, for most getters, we can omit the docstring description and only describe the return value.

I think this is the special case. This is not an ordinary getter. It accesses data storage under the hood and could throw exception, or return error once we get error included in the interface.

Since neither the exception or error are currently in the code, let's write the docstring according to what exists (not what will exist later). When we eventually add the error information, we can update the docstring appropriately.

Add an empty line between methods.

In my experience, it's a bad idea to add code that we'll use in the future since that day may never come. If that day comes, we will remember what to do based on the design doc rather than this commented code.

Return value is missing from docstring.

These docstrings are a bit unclear to me. I guess the idea is that if hard=false, the data can be accessed from any node in the cluster, but if possible, it should be accessed from the nodes specified in this list?

Yes. Hard requirement means data can only be accessed on nodes in the locality list. Will change the docstring to make it clear.

I think it would be clearer to name all these setters as set_xxx where xxx is the thing being set.

How does this class store the data?

Internally all values are serialized and stored in data storage.

You should document template parameters as well.

Our naming convention is to use kebab-case for markdown files except for the main readme in a repo which is called README.md. So this file should be called quick-start.md. I've added this to our internal guidelines.

Our convention is to use sentence case for headings rather than capitlizing every word. So this should be written as # Spider quick start guide.

I've changed all titles, but I treat Spider as a special terminology and keep it capitalized.

Try to use lists rather than a block of text. Lists are faster to read than blocks of text.

I will add a linter to do this later, but treat Markdown as code and wrap lines to 100 characters. This makes it easier to review and read (not everyone has a good Markdown renderer on their local machine).

You have a caveat for starting the worker below. You should note that here otherwise someone will try to start the worker, it won't work, and they will give up.

However, it is impossible to get the return value of the task graph from a client.

This sentence doesn't make sense since the previous section has an example where the client is retrieving the result of a task graph.

Reading the later parts of this guide, I guess you mean that you can't return the value of a dynamically created task to the client (which is intuitive since the interface doesn't provide a mechanism to do so).

This example is complicated and should definitely have a description. To not have a description means that we're slowing down the user since they first need to infer what the goal of the example's code is, then they can focus on the details of using the framework.

This example is also a good reason why our internal guidelines say that we should organize things from public to private. If you apply that thinking here, we would put main first and then the task methods. That is the order that a user should read the methods (the opposite order requires the user to remember a lot of context before they get to main).

input is not a POD, so that violates the constraint mentioned above about task arguments being POD.

We should have an intro section where we describe the actors and architecture in the system (client, scheduler, worker, database, etc.). It doesn't need to be too detailed (that can be in another doc), but it should provide enough context for the user to visualize what we're talking about.

This makes sense, so in the guide above, wouldn't it be easier to just say that we need to create a task library with all the tasks the user wants to run? Then we can reference those tasks in the client code. Right now, the guide says we need to register the tasks by calling spider::register_task which sounds pretty opaque to me.

User is responsible to create a task library and the client executable. Actually a good practice for user is to put the spider::register_task with the tasks' declaration instead of main, so maybe I should task registration into "Create a task section?

1. How do you plan to communicate (serialize) the task calls (name + args) from the client to the server?
2. Let's say a task takes two inputs. Does this interface support using a constant for one input and a task for the other?
   1. If so, do you support a task graph that looks like this?

      flowchart TD
          leaf["foo(int, int) -> int"]
          parent["bar(int, int) -> int"]
          3 --> leaf
          4 --> parent
          5 --> parent
          parent --> leaf
      

      Loading
   2. If so, that means any arguments the user passes into run get passed to the inputs in the task graph with a kind of DFS ordering. Is that true?

1. Here comes the use of spider::register_task, which records the mapping between function name and the function pointer. When the user calls run with the function pointer, the library actually sends the function name to the db. Worker also gets the name of the function as part of the task metadata. Since the worker links to the library with register_task call, it also has the mapping and can know which function to call.
   As for arguments, their types are stored in db (serialized as string right now), and values are serialized into string.
2. Yes.
   i. Yes.
   ii. No. spider::bind needs to bind all inputs of the child task. Thus, the value 3 must be the an argument in bind. 4 and 5 can be passed in run. However, it is possible that there are multiple first layer tasks, i.e. tasks with no parents. The input of task graph is the product of all first layer tasks' output.
   It is possible to support passing 3 as an argument in run for the above example. We can have a special placeholder type for bind. In such case, DFS is an intuitive order.

1. Can you explain how:
   1. register_task maps a function pointer into a function name?
   2. run turns a function pointer into a function name?
2. Can you explain this point with an example?

   However, it is possible that there are multiple first layer tasks, i.e. tasks with no parents. The input of task graph is the product of all first layer tasks' output.

1. For client and worker, when the library is loaded, it will create a mapping between function name and function pointer.
   i. I am thinking of turning register_task into a macro to get the function name at compile time. It then stores the mapping between the function name to the function pointer.
   ii. run gets the function name from the mapping stored before.
2. In the example below, both bar and baz has no parent, and task graph input is their inputs put together, i.e. 1, 2, 3 and 4.

graph TD
    1[1]
    2[2]
    3[3]
    4[4]
    foo["foo(int, int) -> int"]
    bar["bar(int, int) -> int"]
    baz["baz(int, int) -> int"]
    1 --> bar
    2 --> bar
    3 --> baz
    4 --> baz
    bar --> foo
    baz --> foo

1. Makes sense.
2. Gotcha. So to clarify my understanding:
   • in run, users can only pass arguments to root tasks (tasks with no parents).
   • in run, when the user passes a list of arguments [1, 2, 3, 4], conceptually:
     • the framework will iterate over the root tasks in order;
     • for each task argument, the framework will pop one argument from the list.

Yes and yes.

Since this file doesn't correspond to a class, the filename should be lowercase.

We should be using IWYU's pragmas, right?

What will this function do internally?

Let's replace R and Args with C++20 concepts that specify the exact properties of acceptable inputs. If I understand correctly, currently, return values and arguments can be a certain set of serializable values.

On that note though, if the requirement is that the value types need to be serializable, why don't we define an interface for serializable value types and then the user has the flexibility to use any serializable type they want? We could do this later, but conceptually, is it possible?

Let's also use the same C++20 concepts wherever we have task inputs/outputs. This would also simplify the docstrings since we don't need to repeat what types are acceptable.

How about renaming the run functions to start? run implies that the function won't return until the task/taskgraph completes.

Reading the functions in this file and those in KeyValueData, I feel like it makes more sense to have a Client/Driver class that encapsulates the functions (similar to Context). Otherwise, a user could call, for example, insert_kv without initializing a client and then it would fail or invoke undefined behaviour, right?

Double-check all the header guards.

Like I mentioned in Spider.hpp, we can restrict the types using C++20 concepts.

Do we need this anymore considering we've dropped support for building on macOS?

⚠️ Potential issue

Add destructor declaration for pImpl idiom.

When using the pImpl idiom with std::unique_ptr, you need to declare the destructor in the header and define it in the implementation file where the complete type is available.

Add the following declaration:

~Data();  // Declaration only, define in .cpp file

💡 Codebase verification

Template implementation file missing

The code appears to be a header-only template library where:

• Data.hpp in the client directory defines the template interface
• Data.hpp in the core directory contains a different concrete implementation
• The template implementation is missing, and DataImpl is only forward-declared

Required actions:

• Implement the template methods for Data<T> either in the header file or in a separate .ipp file
• Provide the complete implementation of the DataImpl class

#!/bin/bash
# Check for the implementation file
fd "Data\.(cpp|cc|cxx)$" src/spider/client/

# Check if all methods are implemented
rg "Data<.*>::\w+" src/spider/client/

#!/bin/bash
# Let's check if there's a template implementation in the header file itself
rg -U "template.*class Data.*\{.*\}" -A 100 src/spider/client/Data.hpp

# Also check if there's any inline implementation
rg "inline.*Data<.*>::\w+" src/spider/client/

# Check for potential implementation in other directories
fd "Data(Impl)?\.(cpp|cc|cxx|hpp|h)$" src/spider/

#!/bin/bash
# Let's examine the core/Data.hpp to understand if it contains the implementation
cat src/spider/core/Data.hpp

# Also check for DataImpl definition
rg "class DataImpl" src/spider/

This method should throw an exception if the client ID is already in use, right?